dataloader: benchmark + packing-bos-resume strategies - #13
Open
art-test-stack wants to merge 5 commits into
Open
dataloader: benchmark + packing-bos-resume strategies#13art-test-stack wants to merge 5 commits into
art-test-stack wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
scripts/benchmark/dataloaders.py:513
read_documents()returns early oncelimitis reached, skipping the validation that the selected corpus contains text documents. If the firstlimitrows include non-strvalues (e.g., pretokenized lists), this will silently pass and fail later in tokenization.
for row_group in range(parquet.num_row_groups):
documents.extend(parquet.read_row_group(row_group, columns=[column]).column(0).to_pylist())
if len(documents) >= limit:
return documents[:limit]
if not documents or not all(isinstance(value, str) for value in documents):
src/gpt_lab/data/loader.py:582
buffer_size or 1000treats0as falsy and overrides an explicitbuffer_size=0. If0is invalid, it should be rejected explicitly; otherwise, prefer anis Nonecheck so caller intent is preserved.
tokenizer_batch_size=128,
device=dist_info["DEVICE"],
resume_state_dict=resume_state_dict,
buffer_size=buffer_size or 1000,
base_path=(Path(datadir) if datadir is not None else DATA_DIR) / name,
packing_stats=packing_stats,
README.md:205
- This sentence is misleading:
build_dataloaderis exposed from thegpt_lab.datamodule, not “through” theDistDataLoaderclass. Pointing to the module export helps readers find the API quickly.
The `build_dataloader` function, accessible through [`gpt_lab.data.DistDataLoader`](./src/gpt_lab/data/loader.py), exposes three explicit packing strategies:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dataloader packing and benchmark overhaul
Summary
This branch makes token packing explicit, measurable, and resumable. It fixes the default stream loader so tokens and adjacent transitions are preserved across rows and batches, adds a lossless BOS-aligned strategy, retains the legacy destructive best-fit strategy for comparison, and rebuilds the dataloader benchmark around policy-matched correctness and performance measurements.
Packing strategies
streamB*T+1token stream and carries the final token into the next batch.bos_alignedbos_bestfit_cropNeither
streamnorbos_alignedemits padded or incomplete batches. A finite source stops when it cannot fill the next batch.BOS alignment is a layout policy, not document isolation: without segment-aware attention masks, tokens can still attend across document boundaries within a row.
Key changes
DistDataLoaderto preserve FIFO token order, retain long-document tails, and maintain transitions across row and batch boundaries.stream,bos_aligned, andbos_bestfit_cropselection through the public API, configuration, and training CLI.streamandbos_aligned, including pending tokens, carry tokens, continuation state, the active strategy, and packing statistics.PackingStatscounters for source-token flow, destructive crops, skipped transitions, synthetic BOS tokens, intentional BOS boundaries, and buffered state.Benchmark changes
The dataloader benchmark now compares implementations only when they implement the same packing policy. It:
No benchmark result numbers are committed; results depend on the selected corpus, tokenizer, and device.
Test coverage
New tests cover token and transition preservation, long-document tails, destructive-crop accounting, BOS-aligned packing, deterministic validation, exact checkpoint resume, incomplete-batch rejection, benchmark accounting invariants, and warmup exclusion.